From 41579bada5c7091e2be1ece45d7f7e02e62fd6b3 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 21 Nov 2016 12:32:10 -0800 Subject: [PATCH] Apply new fingerprinting to build dir outputs We now much more aggressively cache the output of the compiler based on feature sets and profile configuration. Unfortunately we forgot to extend this caching to build script output directories as well so this commit ensures that build script outputs are cached the same way with a directory per configuration of features and output settings. Closes #3302 --- src/cargo/ops/cargo_clean.rs | 13 +++-- src/cargo/ops/cargo_rustc/context.rs | 62 ++++++++++++++++----- src/cargo/ops/cargo_rustc/custom_build.rs | 22 ++++---- src/cargo/ops/cargo_rustc/fingerprint.rs | 15 ++---- src/cargo/ops/cargo_rustc/layout.rs | 65 ++--------------------- src/cargo/ops/cargo_rustc/mod.rs | 32 +++++------ src/cargo/ops/mod.rs | 4 +- tests/build-script.rs | 46 ++++++++++++++++ 8 files changed, 140 insertions(+), 119 deletions(-) diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 08f9b78c1..fd270b79a 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -73,10 +73,17 @@ pub fn clean(ws: &Workspace, opts: &CleanOptions) -> CargoResult<()> { cx.probe_target_info(&units)?; for unit in units.iter() { - rm_rf(&cx.layout(unit).proxy().fingerprint(&unit.pkg))?; - rm_rf(&cx.layout(unit).build(&unit.pkg))?; + rm_rf(&cx.fingerprint_dir(unit))?; + if unit.target.is_custom_build() { + if unit.profile.run_custom_build { + rm_rf(&cx.build_script_out_dir(unit))?; + } else { + rm_rf(&cx.build_script_dir(unit))?; + } + continue + } - for (src, link_dst, _) in cx.target_filenames(&unit)? { + for (src, link_dst, _) in cx.target_filenames(unit)? { rm_rf(&src)?; if let Some(dst) = link_dst { rm_rf(&dst)?; diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 0d4dc8aef..c0a92d643 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -11,12 +11,12 @@ use std::sync::Arc; use core::{Package, PackageId, PackageSet, Resolve, Target, Profile}; use core::{TargetKind, Profiles, Dependency, Workspace}; use core::dependency::Kind as DepKind; -use util::{CargoResult, ChainError, internal, Config, profile, Cfg, human}; +use util::{self, CargoResult, ChainError, internal, Config, profile, Cfg, human}; use super::TargetConfig; use super::custom_build::{BuildState, BuildScripts}; use super::fingerprint::Fingerprint; -use super::layout::{Layout, LayoutProxy}; +use super::layout::Layout; use super::links::Links; use super::{Kind, Compilation, BuildConfig}; @@ -278,23 +278,61 @@ impl<'a, 'cfg> Context<'a, 'cfg> { } /// Returns the appropriate directory layout for either a plugin or not. - pub fn layout(&self, unit: &Unit) -> LayoutProxy { - let primary = unit.pkg.package_id() == &self.current_package; - match unit.kind { - Kind::Host => LayoutProxy::new(&self.host, primary), - Kind::Target => LayoutProxy::new(self.target.as_ref() - .unwrap_or(&self.host), - primary), + fn layout(&self, kind: Kind) -> &Layout { + match kind { + Kind::Host => &self.host, + Kind::Target => self.target.as_ref().unwrap_or(&self.host) } } + /// Returns the directories where Rust crate dependencies are found for the + /// specified unit. + pub fn deps_dir(&self, unit: &Unit) -> &Path { + self.layout(unit.kind).deps() + } + + /// Returns the directory for the specified unit where fingerprint + /// information is stored. + pub fn fingerprint_dir(&mut self, unit: &Unit) -> PathBuf { + let dir = self.pkg_dir(unit); + self.layout(unit.kind).fingerprint().join(dir) + } + + /// Returns the appropriate directory layout for either a plugin or not. + pub fn build_script_dir(&mut self, unit: &Unit) -> PathBuf { + assert!(unit.target.is_custom_build()); + assert!(!unit.profile.run_custom_build); + let dir = self.pkg_dir(unit); + self.layout(Kind::Host).build().join(dir) + } + + /// Returns the appropriate directory layout for either a plugin or not. + pub fn build_script_out_dir(&mut self, unit: &Unit) -> PathBuf { + assert!(unit.target.is_custom_build()); + assert!(unit.profile.run_custom_build); + let dir = self.pkg_dir(unit); + self.layout(unit.kind).build().join(dir).join("out") + } + /// Returns the appropriate output directory for the specified package and /// target. - pub fn out_dir(&self, unit: &Unit) -> PathBuf { + pub fn out_dir(&mut self, unit: &Unit) -> PathBuf { if unit.profile.doc { - self.layout(unit).doc_root() + self.layout(unit.kind).root().parent().unwrap().join("doc") + } else if unit.target.is_custom_build() { + self.build_script_dir(unit) + } else if unit.target.is_example() { + self.layout(unit.kind).examples().to_path_buf() } else { - self.layout(unit).out_dir(unit) + self.deps_dir(unit).to_path_buf() + } + } + + fn pkg_dir(&mut self, unit: &Unit) -> String { + let name = unit.pkg.package_id().name(); + match self.target_metadata(unit) { + Some(meta) => format!("{}-{}", name, meta), + None => format!("{}-{}", name, util::short_hash(unit.pkg)), } } diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 375a6ea43..eb62f38c5 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -82,11 +82,12 @@ pub fn prepare<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult<(Work, Work)> { - let host_unit = Unit { kind: Kind::Host, ..*unit }; - let (script_output, build_output) = { - (cx.layout(&host_unit).build(unit.pkg), - cx.layout(unit).build_out(unit.pkg)) - }; + let dependencies = cx.dep_run_custom_build(unit)?; + let build_script_unit = dependencies.iter().find(|d| { + !d.profile.run_custom_build && d.target.is_custom_build() + }).expect("running a script not depending on an actual script"); + let script_output = cx.build_script_dir(build_script_unit); + let build_output = cx.build_script_out_dir(unit); // Building the command to execute let to_exec = script_output.join(unit.target.name()); @@ -150,7 +151,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) // This information will be used at build-time later on to figure out which // sorts of variables need to be discovered at that time. let lib_deps = { - cx.dep_run_custom_build(unit)?.iter().filter_map(|unit| { + dependencies.iter().filter_map(|unit| { if unit.profile.run_custom_build { Some((unit.pkg.manifest().links().unwrap().to_string(), unit.pkg.package_id().clone())) @@ -177,8 +178,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) }; cx.build_explicit_deps.insert(*unit, (output_file.clone(), rerun_if_changed)); - fs::create_dir_all(&cx.layout(&host_unit).build(unit.pkg))?; - fs::create_dir_all(&cx.layout(unit).build(unit.pkg))?; + fs::create_dir_all(&script_output)?; + fs::create_dir_all(&build_output)?; // Prepare the unit of "dirty work" which will actually run the custom build // command. @@ -336,9 +337,8 @@ impl BuildOutput { match key { "rustc-flags" => { - let (libs, links) = - BuildOutput::parse_rustc_flags(value, &whence) - ?; + let (libs, links) = + BuildOutput::parse_rustc_flags(value, &whence)?; library_links.extend(links.into_iter()); library_paths.extend(libs.into_iter()); } diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 952ccac78..3d1f27449 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -48,7 +48,7 @@ pub fn prepare_target<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult { let _p = profile::start(format!("fingerprint: {} / {}", unit.pkg.package_id(), unit.target.name())); - let new = dir(cx, unit); + let new = cx.fingerprint_dir(unit); let loc = new.join(&filename(cx, unit)); debug!("fingerprint at: {}", loc.display()); @@ -426,7 +426,7 @@ pub fn prepare_build_cmd<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult { let _p = profile::start(format!("fingerprint build cmd: {}", unit.pkg.package_id())); - let new = dir(cx, unit); + let new = cx.fingerprint_dir(unit); let loc = new.join("build"); debug!("fingerprint at: {}", loc.display()); @@ -507,13 +507,13 @@ fn write_fingerprint(loc: &Path, fingerprint: &Fingerprint) -> CargoResult<()> { debug!("write fingerprint: {}", loc.display()); paths::write(&loc, util::to_hex(hash).as_bytes())?; paths::write(&loc.with_extension("json"), - json::encode(&fingerprint).unwrap().as_bytes())?; + json::encode(&fingerprint).unwrap().as_bytes())?; Ok(()) } /// Prepare work for when a package starts to build pub fn prepare_init(cx: &mut Context, unit: &Unit) -> CargoResult<()> { - let new1 = dir(cx, unit); + let new1 = cx.fingerprint_dir(unit); let new2 = new1.clone(); if fs::metadata(&new1).is_err() { @@ -525,14 +525,9 @@ pub fn prepare_init(cx: &mut Context, unit: &Unit) -> CargoResult<()> { Ok(()) } -/// Return the (old, new) location for fingerprints for a package -pub fn dir(cx: &Context, unit: &Unit) -> PathBuf { - cx.layout(unit).proxy().fingerprint(unit.pkg) -} - /// Returns the (old, new) location for the dep info file of a target. pub fn dep_info_loc(cx: &mut Context, unit: &Unit) -> PathBuf { - dir(cx, unit).join(&format!("dep-{}", filename(cx, unit))) + cx.fingerprint_dir(unit).join(&format!("dep-{}", filename(cx, unit))) } fn compare_old_fingerprint(loc: &Path, new_fingerprint: &Fingerprint) diff --git a/src/cargo/ops/cargo_rustc/layout.rs b/src/cargo/ops/cargo_rustc/layout.rs index cf0cec2df..2b5cd7514 100644 --- a/src/cargo/ops/cargo_rustc/layout.rs +++ b/src/cargo/ops/cargo_rustc/layout.rs @@ -49,10 +49,8 @@ use std::fs; use std::io; use std::path::{PathBuf, Path}; -use core::{Package, Workspace}; +use core::Workspace; use util::{Config, FileLock, CargoResult, Filesystem, human}; -use util::hex::short_hash; -use super::Unit; pub struct Layout { root: PathBuf, @@ -64,11 +62,6 @@ pub struct Layout { _lock: FileLock, } -pub struct LayoutProxy<'a> { - root: &'a Layout, - primary: bool, -} - impl Layout { pub fn new(ws: &Workspace, triple: Option<&str>, @@ -127,58 +120,6 @@ impl Layout { pub fn deps(&self) -> &Path { &self.deps } pub fn examples(&self) -> &Path { &self.examples } pub fn root(&self) -> &Path { &self.root } - - pub fn fingerprint(&self, package: &Package) -> PathBuf { - self.fingerprint.join(&self.pkg_dir(package)) - } - - pub fn build(&self, package: &Package) -> PathBuf { - self.build.join(&self.pkg_dir(package)) - } - - pub fn build_out(&self, package: &Package) -> PathBuf { - self.build(package).join("out") - } - - fn pkg_dir(&self, pkg: &Package) -> String { - format!("{}-{}", pkg.name(), short_hash(pkg)) - } -} - -impl<'a> LayoutProxy<'a> { - pub fn new(root: &'a Layout, primary: bool) -> LayoutProxy<'a> { - LayoutProxy { - root: root, - primary: primary, - } - } - - pub fn root(&self) -> &'a Path { - if self.primary {self.root.dest()} else {self.root.deps()} - } - pub fn deps(&self) -> &'a Path { self.root.deps() } - - pub fn examples(&self) -> &'a Path { self.root.examples() } - - pub fn build(&self, pkg: &Package) -> PathBuf { self.root.build(pkg) } - - pub fn build_out(&self, pkg: &Package) -> PathBuf { self.root.build_out(pkg) } - - pub fn proxy(&self) -> &'a Layout { self.root } - - pub fn out_dir(&self, unit: &Unit) -> PathBuf { - if unit.target.is_custom_build() { - self.build(unit.pkg) - } else if unit.target.is_example() { - self.examples().to_path_buf() - } else { - self.deps().to_path_buf() - } - } - - pub fn doc_root(&self) -> PathBuf { - // the "root" directory ends in 'debug' or 'release', and we want it to - // end in 'doc' instead - self.root.root().parent().unwrap().join("doc") - } + pub fn fingerprint(&self) -> &Path { &self.fingerprint } + pub fn build(&self) -> &Path { &self.build } } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 73f3fed96..c9d3198d2 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -18,7 +18,6 @@ use self::job_queue::JobQueue; pub use self::compilation::Compilation; pub use self::context::{Context, Unit}; -pub use self::layout::{Layout, LayoutProxy}; pub use self::custom_build::{BuildOutput, BuildMap, BuildScripts}; mod compilation; @@ -104,12 +103,6 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>, queue.execute(&mut cx)?; for unit in units.iter() { - let out_dir = cx.layout(unit).build_out(unit.pkg) - .display().to_string(); - cx.compilation.extra_env.entry(unit.pkg.package_id().clone()) - .or_insert(Vec::new()) - .push(("OUT_DIR".to_string(), out_dir)); - for (dst, link_dst, _linkable) in cx.target_filenames(unit)? { let bindst = match link_dst { Some(link_dst) => link_dst, @@ -131,6 +124,13 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>, // Include immediate lib deps as well for unit in cx.dep_targets(unit)?.iter() { + if unit.profile.run_custom_build { + let out_dir = cx.build_script_out_dir(unit).display().to_string(); + cx.compilation.extra_env.entry(unit.pkg.package_id().clone()) + .or_insert(Vec::new()) + .push(("OUT_DIR".to_string(), out_dir)); + } + let pkgid = unit.pkg.package_id(); if !unit.target.is_lib() { continue } if unit.profile.doc { continue } @@ -183,8 +183,7 @@ fn compile<'a, 'cfg: 'a>(cx: &mut Context<'a, 'cfg>, let (dirty, fresh, freshness) = if unit.profile.run_custom_build { custom_build::prepare(cx, unit)? } else { - let (freshness, dirty, fresh) = fingerprint::prepare_target(cx, - unit)?; + let (freshness, dirty, fresh) = fingerprint::prepare_target(cx, unit)?; let work = if unit.profile.doc { rustdoc(cx, unit)? } else { @@ -465,10 +464,6 @@ fn rustdoc(cx: &mut Context, unit: &Unit) -> CargoResult { build_deps_args(&mut rustdoc, cx, unit)?; - if unit.pkg.has_custom_build() { - rustdoc.env("OUT_DIR", &cx.layout(unit).build_out(unit.pkg)); - } - rustdoc.args(&cx.rustdocflags_args(unit)?); let name = unit.pkg.name().to_string(); @@ -624,7 +619,7 @@ fn build_base_args(cx: &mut Context, } -fn build_plugin_args(cmd: &mut ProcessBuilder, cx: &Context, unit: &Unit) { +fn build_plugin_args(cmd: &mut ProcessBuilder, cx: &mut Context, unit: &Unit) { fn opt(cmd: &mut ProcessBuilder, key: &str, prefix: &str, val: Option<&OsStr>) { if let Some(val) = val { @@ -649,15 +644,14 @@ fn build_deps_args(cmd: &mut ProcessBuilder, cx: &mut Context, unit: &Unit) -> CargoResult<()> { cmd.arg("-L").arg(&{ let mut deps = OsString::from("dependency="); - deps.push(cx.layout(unit).deps()); + deps.push(cx.deps_dir(unit)); deps }); - if unit.pkg.has_custom_build() { - cmd.env("OUT_DIR", &cx.layout(unit).build_out(unit.pkg)); - } - for unit in cx.dep_targets(unit)?.iter() { + if unit.profile.run_custom_build { + cmd.env("OUT_DIR", &cx.build_script_out_dir(unit)); + } if unit.target.linkable() && !unit.profile.doc { link_to(cmd, cx, unit)?; } diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index ff4583c56..902c72e30 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -2,8 +2,8 @@ pub use self::cargo_clean::{clean, CleanOptions}; pub use self::cargo_compile::{compile, compile_ws, resolve_dependencies, CompileOptions}; pub use self::cargo_compile::{CompileFilter, CompileMode, MessageFormat}; pub use self::cargo_read_manifest::{read_manifest,read_package,read_packages}; -pub use self::cargo_rustc::{compile_targets, Compilation, Layout, Kind, Unit}; -pub use self::cargo_rustc::{Context, LayoutProxy}; +pub use self::cargo_rustc::{compile_targets, Compilation, Kind, Unit}; +pub use self::cargo_rustc::Context; pub use self::cargo_rustc::{BuildOutput, BuildConfig, TargetConfig}; pub use self::cargo_run::run; pub use self::cargo_install::{install, install_list, uninstall}; diff --git a/tests/build-script.rs b/tests/build-script.rs index e6bcef0fb..dfccd3220 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -2288,3 +2288,49 @@ fn cfg_env_vars_available() { assert_that(build.cargo_process("bench"), execs().with_status(0)); } + +#[test] +fn switch_features_rerun() { + let build = project("builder") + .file("Cargo.toml", r#" + [package] + name = "builder" + version = "0.0.1" + authors = [] + build = "build.rs" + + [features] + foo = [] + "#) + .file("src/main.rs", r#" + fn main() { + println!(include_str!(concat!(env!("OUT_DIR"), "/output"))); + } + "#) + .file("build.rs", r#" + use std::env; + use std::fs::File; + use std::io::Write; + use std::path::Path; + + fn main() { + let out_dir = env::var_os("OUT_DIR").unwrap(); + let out_dir = Path::new(&out_dir).join("output"); + let mut f = File::create(&out_dir).unwrap(); + + if env::var_os("CARGO_FEATURE_FOO").is_some() { + f.write_all(b"foo").unwrap(); + } else { + f.write_all(b"bar").unwrap(); + } + } + "#); + build.build(); + + assert_that(build.cargo("run").arg("-v").arg("--features=foo"), + execs().with_status(0).with_stdout("foo\n")); + assert_that(build.cargo("run").arg("-v"), + execs().with_status(0).with_stdout("bar\n")); + assert_that(build.cargo("run").arg("-v").arg("--features=foo"), + execs().with_status(0).with_stdout("foo\n")); +} -- 2.30.2